Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for data={object} #435

Closed
wants to merge 4 commits into from

Conversation

SanderSpies
Copy link
Contributor

Turns:

var obj = {x:1, t:function(){ return "2"; }};

<p data={obj}/>

into:

<p data-x="1" data-t="2" />

Possible improvement areas:

  • call function with additional parameters (like propTypes)
  • add invariant tests
  • add support for attributes besides data

@chenglou
Copy link
Contributor

data by itself already exists. Wonder if there's any way to circumvent it because this is a pretty neat idea.

@petehunt
Copy link
Contributor

Does dataset work?

@chenglou
Copy link
Contributor

Then this would actually be consistent with things like className. +1 from me!

- changed some variable names to be clearer about they are
@SanderSpies
Copy link
Contributor Author

Should I also add support for aria={object}?

Edit: seems like a bad idea as aria has a limited set of supported variations. Also no relation to the DOM like dataset has.

@sophiebits
Copy link
Contributor

What's a use case for this?

@zpao
Copy link
Member

zpao commented Oct 28, 2013

After talking with @yungsters, I'm not convinced yet that this is a good idea. I think it could be useful if data is used widely, but it still has a pretty minimal usage across the web. If we do decide we want this, there are a number of things about the implementation we'll want to change (this should ultimately work much more like the style prop).

@SanderSpies
Copy link
Contributor Author

Close please.

It would be mainly for consistency with the DOM, which we aren't anyway.
Also, most use cases could be solved with normal React code.

On Monday, October 28, 2013, Paul O’Shannessy wrote:

After talking with @yungsters https://github.com/yungsters, I'm not
convinced yet that this is a good idea. I think it could be useful if
data is used widely, but it still has a pretty minimal usage across the
web. If we do decide we want this, there are a number of things about the
implementation we'll want to change (this should ultimately work much more
like the style prop).


Reply to this email directly or view it on GitHubhttps://github.com//pull/435#issuecomment-27263413
.

@zpao
Copy link
Member

zpao commented Oct 29, 2013

Cool. I'll close it out for now and if we decide in the future we want it, we have something to start from. Thanks for giving it a shot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants